Dispatch beforetoggle & toggle events during dialog open/close/showModal #10091
Conversation
|
I think this needs the "toggle task tracker" concept used in details elements in order to coalesce async toggle events: https://html.spec.whatwg.org/multipage/interactive-elements.html#queue-a-details-toggle-event-task If you do this and open+close the dialog element synchronously , there should be one "toggle" event with both oldState and newState set to "closed": dialog.showModal();
dialog.close(); |
|
Consider chrome as tentatively interested. I will file an intent to ship to get an official position when this gets a bit more attention from the other browsers, but I don't anticipate objections |
|
Also Gecko is interested assuming the pr follows what was discussed at TPAC. |
|
Tests can be added to the OP: web-platform-tests/wpt#44208 |
|
Hi, this implementation could help us improve a popup, such as a CMP, for better integration by utilizing native browser APIs, specifically the Popover API, to make everything more robust. This will enable us to further improve the INP impact, as described in my case study here: https://web.dev/case-studies/pubconsent-inp?hl=en. Since the CMP is a predominant component on the web, I believe this kind of implementation should be supported as much as possible. I hope it helps. Thank you |
|
@annevk would you have time to review this or otherwise delegate it out to another editor for review? |
There was a problem hiding this comment.
Maybe @nt1m or @josepharhar is interested in reviewing this?
source
Outdated
| <li><p>Set <var>element</var>'s <span>dialog toggle task tracker</span> to a struct with <span | ||
| data-x="toggle-task-task">task</span> set to the just-queued <span | ||
| data-x="concept-task">task</span> and <span data-x="toggle-task-old-state">old state</span> set | ||
| to <var>oldState</var>.</p></li> |
There was a problem hiding this comment.
Just-queued is a lil vague. But I guess we have no alternative?
There was a problem hiding this comment.
I cribbed this from the popover part of the spec which does the same thing. Happy to look at alternatives.
There was a problem hiding this comment.
Yeah if all the task queueing algorithms returned the task they create then this would be better. Maybe that could be done as a follow up and we could fix up this part and the identical popover one?
There was a problem hiding this comment.
Either that or create the task ahead-of-time. Not sure what is better, but a follow-up issue seems reasonable.
There was a problem hiding this comment.
I will work on a follow up PR to fix both callsites.
whatwg/html#10091 This also slightly refactors FireToggleEvent to allow to to accomodate both popovers and now also dialogs Differential Revision: https://phabricator.services.mozilla.com/D225449 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1876762 gecko-commit: a8b51403fa4f1dd6d936826f2a5b14c670426f3d gecko-reviewers: smaug
…e r=smaug whatwg/html#10091 This also slightly refactors FireToggleEvent to allow to to accomodate both popovers and now also dialogs Differential Revision: https://phabricator.services.mozilla.com/D225449
whatwg/html#10091 This also slightly refactors FireToggleEvent to allow to to accomodate both popovers and now also dialogs Differential Revision: https://phabricator.services.mozilla.com/D225449 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1876762 gecko-commit: a8b51403fa4f1dd6d936826f2a5b14c670426f3d gecko-reviewers: smaug
…e r=smaug whatwg/html#10091 This also slightly refactors FireToggleEvent to allow to to accomodate both popovers and now also dialogs Differential Revision: https://phabricator.services.mozilla.com/D225449 UltraBlame original commit: a8b51403fa4f1dd6d936826f2a5b14c670426f3d
…e r=smaug whatwg/html#10091 This also slightly refactors FireToggleEvent to allow to to accomodate both popovers and now also dialogs Differential Revision: https://phabricator.services.mozilla.com/D225449 UltraBlame original commit: a8b51403fa4f1dd6d936826f2a5b14c670426f3d
…e r=smaug whatwg/html#10091 This also slightly refactors FireToggleEvent to allow to to accomodate both popovers and now also dialogs Differential Revision: https://phabricator.services.mozilla.com/D225449 UltraBlame original commit: a8b51403fa4f1dd6d936826f2a5b14c670426f3d
|
lgtm, I don't see any differences between this spec and the chromium implementation |
|
I wonder if one of the editors would like to review this, given it'll be shipping in 2 browsers very soon? Perhaps @foolip, @domfarolino or @zcorpan? |
source
Outdated
| <li><p>Set <var>element</var>'s <span>dialog toggle task tracker</span> to a struct with <span | ||
| data-x="toggle-task-task">task</span> set to the just-queued <span | ||
| data-x="concept-task">task</span> and <span data-x="toggle-task-old-state">old state</span> set | ||
| to <var>oldState</var>.</p></li> |
There was a problem hiding this comment.
Either that or create the task ahead-of-time. Not sure what is better, but a follow-up issue seems reasonable.
keithamus
force-pushed
the
dispatch-toggleevents-for-dialog-open-close
branch
from
2a3abd6
to
b666d93
Compare
Corresponds to whatwg/html#10091 (cherry picked from commit 36f8dfaed02a967de620a7e2abc8bd254cd7f9a5)
Corresponds to whatwg/html#10091 (cherry picked from commit 36f8dfaed02a967de620a7e2abc8bd254cd7f9a5)
Corresponds to whatwg/html#10091 (cherry picked from commit 36f8dfaed02a967de620a7e2abc8bd254cd7f9a5)
This attempts to fix #9733 by specifying that:
show()steps should dispatch abeforetogglecancellable event.toggleevent.showModal()steps should dispatch abeforetogglecancellable event.toggleevent.close the dialogsteps should disaptch a non-cancellablebeforetoggleevent.toggleevent.(See WHATWG Working Mode: Changes for more details.)
/interactive-elements.html ( diff )